Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not encode gensymed imports in metadata #59296

Merged
merged 1 commit into from
Mar 24, 2019
Merged

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Mar 19, 2019

(Unless they are underscore _ imports which are re-gensymed on crate loading, see #56392.)

We cannot encode gensymed imports properly in metadata and if we encode them improperly, we can get erroneous name conflicts downstream.
Gensymed imports are produced by the compiler, so we control their set, and can be sure that none of them needs being encoded for use from other crates.

A workaround that fixes #59243.

@rust-highfive
Copy link
Collaborator

r? @estebank

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 19, 2019
@@ -312,7 +312,7 @@ impl<'a> Resolver<'a> {
Ident::new(keywords::SelfLower.name(), new_span)
),
kind: ast::UseTreeKind::Simple(
Some(Ident::new(keywords::Underscore.name().gensymed(), new_span)),
Some(Ident::new(Name::gensym("__dummy"), new_span)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this shift the problem to the possible existence of __dummy somewhere in the namespace?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's still a gensym, so it doesn't conflict with anything locally + it's not written into metadata since it's no longer an underscore.
(Previously it wasn't written due to the binding.vis != ty::Visibility::Invisible condition, but I removed it to simplify things a bit.)

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Mar 19, 2019

📌 Commit 30d5dc9 has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 19, 2019
Centril added a commit to Centril/rust that referenced this pull request Mar 19, 2019
Do not encode gensymed imports in metadata

(Unless they are underscore `_` imports which are re-gensymed on crate loading, see rust-lang#56392.)

We cannot encode gensymed imports properly in metadata and if we encode them improperly, we can get erroneous name conflicts downstream.
Gensymed imports are produced by the compiler, so we control their set, and can be sure that none of them needs being encoded for use from other crates.

A workaround that fixes rust-lang#59243.
kennytm added a commit to kennytm/rust that referenced this pull request Mar 24, 2019
Do not encode gensymed imports in metadata

(Unless they are underscore `_` imports which are re-gensymed on crate loading, see rust-lang#56392.)

We cannot encode gensymed imports properly in metadata and if we encode them improperly, we can get erroneous name conflicts downstream.
Gensymed imports are produced by the compiler, so we control their set, and can be sure that none of them needs being encoded for use from other crates.

A workaround that fixes rust-lang#59243.
bors added a commit that referenced this pull request Mar 24, 2019
Rollup of 7 pull requests

Successful merges:

 - #59213 (Track changes to robots.txt)
 - #59239 (Remove inline assembly from hint::spin_loop)
 - #59251 (Use a valid name for graphviz graphs)
 - #59296 (Do not encode gensymed imports in metadata)
 - #59328 (Implement specialized nth_back() for Box and Windows.)
 - #59355 (Fix ICE with const generic param in struct)
 - #59377 (Correct minimum system LLVM version in tests)
@bors bors merged commit 30d5dc9 into rust-lang:master Mar 24, 2019
@petrochenkov petrochenkov deleted the stdup branch June 5, 2019 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

the name std is defined multiple times
4 participants